[Updated PR] [Feature] Autocomplete context from other tabs open in the editor #6012
Conversation
…te cache element recency
…reak from this branch to test changeVisibleTextEditors
…aded into context
…context provision
… open file viewing
|
Your cubic subscription is currently inactive. Please reactivate your subscription to receive AI reviews and use cubic. |
✅ Deploy Preview for continuedev canceled.
|
dosubot
bot
added
the
size:L
There was a problem hiding this comment.
Clean implementation!
Some changes in mind:
- Use the existing
files/closedorfiles/openedmessages rather than introducing a newonDidCloseTextDocumentcallback andinitializeOpenedFileCachemessage. "files/closed" is already set up for both vs code and jetbrains. It seems like in this implementation,recentlyEditedis for open files andrecentlyOpenedis for closed files, butrecentlyOpenedis also initialized with open files. Let's talk about this naming and review the snippet priority. I think recently edited should be prioritized over recently closed. Also, let's implement the existingfiles/openedmessage rather than adding a newinitializeOpenedFileCachemessage. It seems like those should be added to the recently edited cache on startup, not the recently "opened" (closed) cache. - Use URIs all the way through, don't use "file://", will cause bugs, just use uri.toString() in the IDE side . Note that
helper.filepathwas named before we transitioned to URIs, bit misleading - Let's add unit tests for the formatting and ranking, especially around the math stuff!
I also wanted to talk to you about snippet ranking, deduplication, and file read latency, can chat in person, you probably already tested this enough but wanted to get a grasp on it. Did we consider storing the actual file contents in the cache? Might not be up to date but would be much faster and these are for "closed" files. Files are often large, is there a concern of recently closed files always filling the remainder of snippet context window?
Were the additions/changes to manual-testing-sandbox intentional? Fine to be included, just checking
EDIT:
after talking with @adarshramiyer feedback has changed to
- Use the existing "files/closed" message rather than a
onDidCloseTextDocumentcallback (partly so works with jetbrains), move logic to core files/closed - Instead of a
initializeOpenedFileCachemessage, send a "files/opened" message on init, switch onDidChangeActive to using the "files/opened" - Rip out the recentlyEditedFiles cache since it's duplicate
- Use URIs all the way through, no fsPath or "file://" templating
- Promise.all for parallel file reads, and race each one to maybe 80 ms, something a bit less than 100
- Deduplication of snippets with e.g. recently visited ranges, but can be done on a future PR
extensions/vscode/src/VsCodeIde.ts
Outdated
| let openFilePaths = vscode.window.tabGroups.all // get tab groups | ||
| .flatMap((group) => group.tabs) // extract tabs from tab groups | ||
| .filter((tab) => tab.input instanceof vscode.TabInputText) // filter irrelevant tabs (settings, etc.) | ||
| .map((tab) => (tab.input as vscode.TabInputText).uri.fsPath); // get filepaths |
There was a problem hiding this comment.
Use the existing VsCodeIDEUtils getOpenFiles function. Looks like you might be able to improve it by using this TabInputText approach instead of the existing documentIsCode filter. Make sure to use uri.toString() instead of fsPath
| } | ||
|
|
||
| try { | ||
| const fileContent = await ide.readFile(fileUri); |
There was a problem hiding this comment.
How was the max cache size decided, by file read latency? It seems like with this setup it might often not finish reading all files in the cache within 100 ms, e.g. with 20 files could take 200+. Consider using Promise.all()
core/core.ts
Outdated
| async ({ data: { initialOpenedFilePaths } }) => { | ||
| try { | ||
| for (const filepath of initialOpenedFilePaths) { | ||
| openedFilesLruCache.set(`file://${filepath}`, `file://${filepath}`); |
There was a problem hiding this comment.
remove file://, use URI from IDE all the way through
|
|
||
| // Returns linear combination of recency and size scores | ||
| // recency score is exponential decay over recency; log normalized score is used for size | ||
| const getRecencyAndSizeScore = ( |
There was a problem hiding this comment.
This would be awesome for unit tests
dosubot
bot
removed
the
size:L
dosubot
bot
added
the
size:XL
core/core.ts
Outdated
| @@ -597,14 +600,56 @@ export class Core { | |||
| }); | |||
|
|
|||
| on("files/closed", async ({ data }) => { | |||
| try { | |||
| const filepaths = data.fileUris; | |||
There was a problem hiding this comment.
No need for fileUris and uris, same thing
| @@ -76,6 +78,7 @@ export class VsCodeExtension { | |||
| }, | |||
| ); | |||
| this.ide = new VsCodeIde(this.webviewProtocolPromise, context); | |||
| this.ideUtils = new VsCodeIdeUtils(); | |||
There was a problem hiding this comment.
use this.ide.getOpenFiles and merge the utils, no need for new util
…ontinuedev/continue into feature/view-context/base-copy
Description
Adds context to autocomplete using files open in the editor. Logs open files and viewing order in an LRU cache, then prioritizes fitting recent and shorter files into the context window. Adaptively trims files to fit within prompt token limits as necessary.
Checklist